H1: add validated feed primitive status - #41
Conversation
Co-Authored-By: Codex <noreply@openai.com>
🤖 Codex PR Review🚫 Merge blocked: 2 serious issue(s) found in high-risk files ⚖️ Codex Review Arbitration🚫 block: 🚫 Blocking IssuesThese issues must be fixed before this PR can be merged: 1. 🟠 [HIGH] Logic in
|
Co-Authored-By: Codex <noreply@openai.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d3b0e66553
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| item_count=0, | ||
| error=f"{type(exc).__name__}: {exc}", | ||
| ) | ||
| error_code = exc.code if isinstance(exc, (FeedXmlError, PrimitiveStatusError)) else "fetch_failed" |
There was a problem hiding this comment.
When parse_feed_items raises FeedXmlError for invalid or oversized XML while --continue-on-feed-error is set, this line enters the isinstance branch and reads exc.code, but FeedXmlError only carries its string message and has no code attribute. The exception handler then raises AttributeError before appending a failed record or continuing, so one malformed feed can still abort the RSS workflow.
Useful? React with 👍 / 👎.
| output_path = Path(path) | ||
| output_path.parent.mkdir(parents=True, exist_ok=True) | ||
| output_path.write_text(json.dumps(payload, ensure_ascii=False, indent=2, sort_keys=True) + "\n", encoding="utf-8") | ||
| output_path.write_bytes(payload + b"\n") |
There was a problem hiding this comment.
The new status parser enforces canonical bytes by checking serialize_status(parsed) == wire, so the trailing newline written here makes every producer status file fail parse_status_bytes/status_for_rows with status_noncanonical when read back as bytes. If these files are meant to be the canonical contract output, write exactly payload or make the parser explicitly allow the file terminator.
Useful? React with 👍 / 👎.
| if any(type(snapshot[key]) is not int or snapshot[key] < 0 for key in integer_keys): | ||
| raise _fail("status_counter_invalid") |
There was a problem hiding this comment.
These checks only require counters to be nonnegative Python ints, so a crafted status with matching top-level and per-feed accepted_row_count values such as 9007199254740993 is accepted and re-serialized as canonical even though that value is not safe for JSON consumers that preserve integers only up to 2^53. Since this parser is the boundary for the canonical status contract, it should reject counters outside the intended safe range instead of blessing them.
Useful? React with 👍 / 👎.
| if state == "quarantined" and not error_code: | ||
| raise _fail("feed_state_invalid") |
There was a problem hiding this comment.
A caller can pass a quarantined feed with an error_code and non-empty rows, and this invariant accepts it; _feed_wire then reports those rows as accepted_row_count and includes them in the aggregate row digest even though the feed is not publication-eligible. If quarantined rows are meant to be withheld or rejected, this lets invalid primitive records be blessed by the public status builder.
Useful? React with 👍 / 👎.
1 participant
Scope
pert.feed_primitives.v1status contract.Contract
defusedxmlboundary.accepted,failed, andquarantined; stale/missing are not invented.Verification
uv sync --locked --extra testpasseduv run pytest -q— 135 passedpython3 -m compileall -q src scripts testspassedgit diff --checkpassedruffunavailable in the environmentNo workflow, publish, QAR, weekly artifact, permission, or automation changes. W0-B remains deferred until this PR merges and post-merge CI succeeds.